Skip to content

Conversation

@kdhttps
Copy link
Contributor

@kdhttps kdhttps commented Oct 21, 2025

close #2313

Summary by CodeRabbit

  • New Features

    • Cedarling configuration page under Security to manage policy store URL, retrieval point, set-as-default, and trigger mappings sync; multilingual texts added (EN/ES/FR/PT).
  • Changes

    • Security menu label updated to "Roles and Permissions". Mapping list simplified to a read-only view linking to Cedarling config; added sync/refresh actions and inline documentation notes.
  • Removed

    • Legacy roles/permissions management pages, add/edit dialog UIs, and related unit tests.
  • Documentation

    • Minor formatting update to admin configuration docs.

@kdhttps kdhttps self-assigned this Oct 21, 2025
@mo-auto mo-auto added comp-admin-ui Component affected by issue or PR kind-feature Issue or PR is a new feature request labels Oct 21, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 30, 2025

📝 Walkthrough

Walkthrough

Adds a Cedarling admin configuration page and translations, registers an apiConfig Redux slice and audit resource, updates plugin menu/routes, simplifies mapping UI, and removes several legacy role/permission components and their tests.

Changes

Cohort / File(s) Summary
Translations
admin-ui/app/locales/*/translation.json
\en`, `es`, `fr`, `pt``
Added Cedarling-related keys (cedarlingConfig, cedarling_config, auiPolicyStoreUrl, configApiPolicyStoreUrl, cedarlingPolicyStoreRetrievalPoint) and SAML/authn/mappings/documentation entries across locales.
Cedarling config page
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
New React page component: reads and edits admin-ui config, toggles retrieval point, performs authorization checks, calls mutations (edit/set default), triggers role-to-scope sync, logs audit events, manages loading/toasts, and invalidates queries.
Redux slice & audit resource
admin-ui/plugins/admin/redux/features/apiConfigSlice.js
admin-ui/plugins/admin/redux/audit/Resources.tsx
Added apiConfig slice with editConfig action and exported reducer; added ADMIN_UI_CEDARLING_CONFIG audit resource constant.
Plugin metadata / routing
admin-ui/plugins/admin/plugin-metadata.ts
Registered apiConfigReducer, added Cedarling menu entry and route /adm/cedarlingconfig, wired CedarlingConfigPage, and removed legacy role pages from menu/routes.
Mapping UI simplification
admin-ui/plugins/admin/components/Mapping/MappingItem.js
admin-ui/plugins/admin/components/Mapping/MappingPage.js
Simplified MappingItem to read-only display; removed add-mapping dialog and add-flow UI, replaced with Cedarling configuration note/link.
Removed role/permission UI components
admin-ui/plugins/admin/components/Roles/*.js
admin-ui/plugins/admin/components/Permissions/*.js
Deleted legacy role/permission list/detail/add components and add-dialog components (UiRoleListPage, UiRoleDetailPage, RoleAddDialogForm, UiPermListPage, UiPermDetailPage, PermissionAddDialogForm).
Removed tests
admin-ui/plugins/admin/__tests__/components/Roles/UiRoleListPage.test.js
admin-ui/plugins/admin/__tests__/components/Roles/UiRoleDetailPage.test.js
Removed unit tests associated with deleted role components.
Documentation
docs/admin/admin-ui/configuration.md
Minor formatting tweak to code block fence (no semantic change).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant UI as CedarlingConfigPage
    participant Auth as Authorization
    participant API as AdminUI Config API
    participant Sync as RoleMapping Sync
    participant Store as QueryCache
    participant Audit as AuditService

    User->>UI: Open page / Submit form
    UI->>Auth: authorize READ/WRITE/DELETE
    alt unauthorized
        Auth-->>UI: deny
        UI-->>User: show permission error
    else authorized
        UI->>API: editAdminuiConf(payload)
        API-->>UI: updated config
        UI->>Store: invalidate adminui config query
        UI->>Audit: log ADMIN_UI_CEDARLING_CONFIG (update)
        UI->>Sync: trigger role-to-scope sync
        Sync-->>UI: ack/result
        UI-->>User: show success or error
    end

    User->>UI: Click "Set remote default"
    UI->>API: setRemotePolicyStoreAsDefault()
    API-->>UI: result
    UI->>Audit: log ADMIN_UI_CEDARLING_CONFIG (set default)
    UI-->>User: show success/error
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Pay special attention to:
    • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx — permission checks, mutation flows, audit payloads, query invalidation.
    • admin-ui/plugins/admin/plugin-metadata.ts — reducer registration, route/menu permission mapping, removal of legacy entries.
    • admin-ui/plugins/admin/redux/features/apiConfigSlice.js — state shape and merge logic for editConfig.
    • Ensure no remaining imports/usages of removed role/permission components and update tests accordingly.
    • Translation keys parity across all locales.

Possibly related issues

  • #2310 — Parent Cedarling integration issue; this PR implements the Admin UI page and config keys referenced by that objective.
  • #2313 — Feature request to add a Cedarling configuration page; this PR provides the UI described.

Possibly related PRs

Poem

🐰
I hopped into code with a curious grin,
Cedarling keys and pages tucked in,
Mappings point where policies steer,
I stitched a config page — a tiny cheer,
Hooray, the rabbit's change is here!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a UI page for Cedarling configuration in the admin-ui, which is the primary focus of the PR.
Linked Issues check ✅ Passed The PR implements the Cedarling configuration UI page with component, routing, translations, and state management, aligning with the linked issue #2313 objective to create this UI feature.
Out of Scope Changes check ✅ Passed The PR removes obsolete role and permission management components (UiRoleListPage, UiPermListPage, related forms/details) alongside adding Cedarling config, which aligns with transitioning to the new Cedarling-based architecture.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-2313-cedarling-ui

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c3e75a1 and e529e1b.

📒 Files selected for processing (2)
  • admin-ui/app/locales/en/translation.json (4 hunks)
  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).

Applied to files:

  • admin-ui/app/locales/en/translation.json
  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.

Applied to files:

  • admin-ui/app/locales/en/translation.json
  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.

Applied to files:

  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.

Applied to files:

  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (4)
admin-ui/plugins/admin/components/Mapping/MappingPage.js (1)
  • useCedarling (22-22)
admin-ui/app/utils/AuditLogger.ts (1)
  • logAudit (68-68)
admin-ui/plugins/admin/redux/audit/Resources.tsx (1)
  • ADMIN_UI_CEDARLING_CONFIG (5-5)
admin-ui/app/utils/PermChecker.ts (3)
  • PROPERTIES_READ (3-3)
  • PROPERTIES_WRITE (4-4)
  • PROPERTIES_DELETE (5-5)
🔇 Additional comments (6)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (6)

1-41: LGTM! Well-organized imports.

The imports are comprehensive and properly organized, including all necessary dependencies for the Cedarling configuration feature.


42-67: LGTM! Proper state and hook initialization.

The component correctly initializes all necessary hooks and state variables. Redux selectors are properly typed, and the default value for cedarlingPolicyStoreRetrievalPoint appropriately matches the expected type.


68-124: LGTM! Comprehensive form submission handler.

The handleSubmit function is well-structured with:

  • Proper validation for the required URL field
  • Correct state updates from the mutation response
  • Appropriate audit logging for both config update and sync operations (including requestData in the sync audit payload as per learnings)
  • Good error handling with user-facing toasts and developer console logs
  • Query invalidation to ensure UI consistency

Based on learnings


126-152: LGTM! Appropriate handling of the refresh action.

The handleSetRemotePolicyStoreAsDefault function correctly handles the policy store refresh operation. Since this mutation only performs a server-side action without modifying configuration values (as clarified in past discussions), it appropriately doesn't update local state or invalidate queries.

Based on learnings


154-164: LGTM! Proper initialization effects.

Both useEffect hooks are well-structured:

  • Permission checks run in parallel using Promise.allSettled with proper dependency tracking
  • Configuration state is hydrated from the fetched data, initializing both auiPolicyStoreUrl and cedarlingPolicyStoreRetrievalPoint

The dependency array [isSuccess] in the second effect is acceptable as auiConfig from react-query changes when isSuccess changes, avoiding unnecessary re-runs.


166-278: LGTM! Well-structured UI implementation.

The JSX render is properly organized with:

  • Appropriate loading states via GluuLoader
  • Clear documentation and instructions for users with external links (properly secured with rel="noopener noreferrer")
  • Controlled form inputs with proper validation (HTML5 type="url")
  • Logical visibility rules for the refresh button (hidden when in 'remote' mode or URL is empty)
  • Proper radio button implementation using Material-UI's controlled RadioGroup

The hardcoded radio labels are intentional pending term finalization, as discussed in previous reviews.

Based on learnings


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kdhttps kdhttps marked this pull request as ready for review November 3, 2025 18:24
@kdhttps
Copy link
Contributor Author

kdhttps commented Nov 3, 2025

I think we should make another PR for Cedarling integration. Otherwise, there will be big code changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c3cc6b and 501dc0b.

📒 Files selected for processing (8)
  • admin-ui/app/locales/en/translation.json (4 hunks)
  • admin-ui/app/locales/es/translation.json (4 hunks)
  • admin-ui/app/locales/fr/translation.json (4 hunks)
  • admin-ui/app/locales/pt/translation.json (4 hunks)
  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1 hunks)
  • admin-ui/plugins/admin/plugin-metadata.ts (5 hunks)
  • admin-ui/plugins/admin/redux/audit/Resources.tsx (1 hunks)
  • admin-ui/plugins/admin/redux/features/apiConfigSlice.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (3)
admin-ui/app/utils/AuditLogger.ts (1)
  • logAudit (68-68)
admin-ui/plugins/admin/components/MAU/MauGraph.js (1)
  • initPermissions (36-38)
admin-ui/app/utils/PermChecker.ts (3)
  • PROPERTIES_READ (3-3)
  • PROPERTIES_WRITE (4-4)
  • PROPERTIES_DELETE (5-5)
admin-ui/plugins/admin/plugin-metadata.ts (1)
admin-ui/app/utils/PermChecker.ts (1)
  • MAPPING_READ (26-27)

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Caution

Docstrings generation - FAILED

No docstrings were generated.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

♻️ Duplicate comments (2)
admin-ui/plugins/admin/redux/features/apiConfigSlice.js (1)

45-46: Remove non-standard state export.

Exporting slice.state leaks initialState and confuses consumers. Use selectors instead.

-export const { actions, reducer, state } = apiConfigSlice
+export const { actions, reducer } = apiConfigSlice
admin-ui/plugins/admin/plugin-metadata.ts (1)

100-104: Align Cedarling menu permission with page requirements.

Gate on PROPERTIES_READ to avoid immediate auth failure on navigation.

-              permission: MAPPING_READ,
+              permission: PROPERTIES_READ,
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 501dc0b and 0e49977.

📒 Files selected for processing (7)
  • admin-ui/app/locales/en/translation.json (4 hunks)
  • admin-ui/app/locales/es/translation.json (4 hunks)
  • admin-ui/app/locales/fr/translation.json (4 hunks)
  • admin-ui/app/locales/pt/translation.json (4 hunks)
  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1 hunks)
  • admin-ui/plugins/admin/plugin-metadata.ts (5 hunks)
  • admin-ui/plugins/admin/redux/features/apiConfigSlice.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
admin-ui/plugins/admin/plugin-metadata.ts (1)
admin-ui/app/utils/PermChecker.ts (2)
  • MAPPING_READ (26-27)
  • PROPERTIES_READ (3-3)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (3)
admin-ui/app/utils/AuditLogger.ts (1)
  • logAudit (68-68)
admin-ui/plugins/admin/components/MAU/MauGraph.js (1)
  • initPermissions (36-38)
admin-ui/app/utils/PermChecker.ts (3)
  • PROPERTIES_READ (3-3)
  • PROPERTIES_WRITE (4-4)
  • PROPERTIES_DELETE (5-5)
🔇 Additional comments (12)
admin-ui/app/locales/pt/translation.json (2)

65-67: LGTM: menu entry added.

Cedarling menu key and placement look consistent across locales.


652-655: LGTM: Cedarling config keys present and consistent.

Fields auiPolicyStoreUrl/configApiPolicyStoreUrl and documentation.cedarlingConfig.note/useRemotePolicyStore match the component’s lookups.

Also applies to: 857-857, 1590-1596

admin-ui/plugins/admin/plugin-metadata.ts (1)

211-211: Reducer registration duplication check.

apiConfigReducer is registered here; ensure the slice file does not also register itself to prevent duplicates. See suggested change in apiConfigSlice.js.

admin-ui/app/locales/fr/translation.json (1)

657-660: LGTM: keys match component usage.

New fields and documentation keys align with CedarlingConfigPage lookups.

Also applies to: 869-870, 1597-1602

admin-ui/app/locales/es/translation.json (4)

623-625: ✓ Looks good.

The three new field translation keys are properly added with clear Spanish translations. No issues detected.


658-659: ✓ Menu entries are consistent.

The menu dropdown additions for "mapping" and "cedarlingConfig" with their Spanish translations are properly placed and consistent with the English locale.


971-971: ✓ Title translation is appropriate.

The cedarling_config title with Spanish translation follows the established pattern.


1741-1747: The review comment is incorrect.

useRemotePolicyStore is intentionally camelCase because it's a field name reference that maps to the actual configuration field used in the codebase. This is confirmed by:

  1. Redux state reference in apiConfigSlice.js (line 27): useRemotePolicyStore: true is a state field
  2. Component usage in CedarlingConfigPage.tsx (line 178): The tooltip explicitly references it as doc_entry={'useRemotePolicyStore'}, which must match the actual field name

The other keys (title, point1, point2, note) are UI descriptive strings, not field references, so their lowercase naming doesn't apply to useRemotePolicyStore. The naming is consistent across all locale files (en, fr, pt, es) and serves a functional purpose.

Likely an incorrect or invalid review comment.

admin-ui/app/locales/en/translation.json (4)

623-625: ✓ Field translations are clear.

The three new field keys with English translations are properly formatted and consistent with their Spanish counterparts.


658-659: ✓ Menu entries match Spanish locale.

The security dropdown menu additions are properly formatted and consistent with the Spanish translation file.


978-978: ✓ Title translation is clear.

The cedarling_config title follows the established pattern consistently.


1748-1754: No action required - review comment is incorrect.

The useRemotePolicyStore key uses camelCase intentionally because it references the actual configuration field name defined in apiConfigSlice.js (line 27) and used in CedarlingConfigPage.tsx (lines 59, 178). The tooltip lookup explicitly uses doc_entry={'useRemotePolicyStore'} to retrieve the translation, confirming the naming is intentional and necessary. Renaming to use_remote_policy_store would break this connection to the backend configuration field. This naming pattern is consistent across all four locale files (en, es, fr, pt).

Likely an incorrect or invalid review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0e49977 and febcfc3.

📒 Files selected for processing (5)
  • admin-ui/app/locales/en/translation.json (4 hunks)
  • admin-ui/app/locales/fr/translation.json (4 hunks)
  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1 hunks)
  • admin-ui/plugins/admin/redux/audit/Resources.tsx (1 hunks)
  • admin-ui/plugins/admin/redux/features/apiConfigSlice.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (3)
admin-ui/app/utils/AuditLogger.ts (1)
  • logAudit (68-68)
admin-ui/plugins/admin/redux/audit/Resources.tsx (1)
  • ADMIN_UI_CONFIG (6-6)
admin-ui/app/utils/PermChecker.ts (3)
  • PROPERTIES_READ (3-3)
  • PROPERTIES_WRITE (4-4)
  • PROPERTIES_DELETE (5-5)

Copy link
Contributor

@duttarnab duttarnab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see inline comments

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1)

156-161: Add auiConfig to useEffect dependency array.

The effect reads auiConfig but only includes isSuccess in the dependency array. This violates React's exhaustive-deps rule and can cause stale closures. If auiConfig changes while isSuccess remains true, the effect won't re-run to hydrate the new values, leaving the UI displaying outdated configuration.

Apply this diff to fix the dependency array:

   useEffect(() => {
     if (isSuccess) {
       setAuiPolicyStoreUrl(auiConfig?.auiPolicyStoreUrl || '')
       setCedarlingPolicyRetrievalPoint(auiConfig?.cedarlingPolicyStoreRetrievalPoint || 'remote')
     }
-  }, [isSuccess])
+  }, [isSuccess, auiConfig])
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e84df41 and 4b5cf15.

📒 Files selected for processing (1)
  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).

Applied to files:

  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.

Applied to files:

  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.

Applied to files:

  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.

Applied to files:

  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (4)
admin-ui/plugins/admin/components/Mapping/MappingPage.js (1)
  • useCedarling (22-22)
admin-ui/app/utils/AuditLogger.ts (1)
  • logAudit (68-68)
admin-ui/plugins/admin/redux/audit/Resources.tsx (1)
  • ADMIN_UI_CEDARLING_CONFIG (5-5)
admin-ui/app/utils/PermChecker.ts (3)
  • PROPERTIES_READ (3-3)
  • PROPERTIES_WRITE (4-4)
  • PROPERTIES_DELETE (5-5)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1)

159-164: Add missing dependency to useEffect.

The effect reads auiConfig but doesn't list it in the dependency array, which violates the React hooks exhaustive-deps rule. This can cause stale closures where the effect uses an outdated version of auiConfig.

Apply this diff:

   useEffect(() => {
     if (isSuccess) {
       setAuiPolicyStoreUrl(auiConfig?.auiPolicyStoreUrl || '')
       setCedarlingPolicyRetrievalPoint(auiConfig?.cedarlingPolicyStoreRetrievalPoint || 'remote')
     }
-  }, [isSuccess])
+  }, [isSuccess, auiConfig])
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b5cf15 and 59af644.

📒 Files selected for processing (1)
  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).

Applied to files:

  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.

Applied to files:

  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.

Applied to files:

  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.

Applied to files:

  • admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (4)
admin-ui/plugins/admin/components/Mapping/MappingPage.js (3)
  • useCedarling (22-22)
  • useTranslation (20-20)
  • dispatch (19-19)
admin-ui/app/utils/AuditLogger.ts (1)
  • logAudit (68-68)
admin-ui/plugins/admin/redux/audit/Resources.tsx (1)
  • ADMIN_UI_CEDARLING_CONFIG (5-5)
admin-ui/app/utils/PermChecker.ts (3)
  • PROPERTIES_READ (3-3)
  • PROPERTIES_WRITE (4-4)
  • PROPERTIES_DELETE (5-5)
🔇 Additional comments (1)
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1)

154-157: Verify permission gating strategy.

The permission checks run on mount but don't gate form rendering. Users without proper permissions will see the form and only encounter errors when attempting actions. Consider whether you want to:

  • Show a loading state until permissions are verified
  • Conditionally render the form based on permission results
  • Disable form controls for users lacking write permissions

If the current behavior is intentional (showing the form to all users), this comment can be disregarded.

@sonarqubecloud
Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12c6407 and 0979d9b.

📒 Files selected for processing (4)
  • admin-ui/app/locales/en/translation.json (4 hunks)
  • admin-ui/app/locales/es/translation.json (4 hunks)
  • admin-ui/app/locales/fr/translation.json (4 hunks)
  • admin-ui/app/locales/pt/translation.json (4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).

Applied to files:

  • admin-ui/app/locales/fr/translation.json
  • admin-ui/app/locales/pt/translation.json
  • admin-ui/app/locales/en/translation.json
  • admin-ui/app/locales/es/translation.json
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.

Applied to files:

  • admin-ui/app/locales/fr/translation.json
  • admin-ui/app/locales/en/translation.json
🔇 Additional comments (3)
admin-ui/app/locales/pt/translation.json (1)

1592-1603: Ensure Portuguese translations match English baseline and address incomplete sentence in note field.

The cedarlingConfig section in Portuguese should align with the English file (lines 1750-1761). Line 1596 ("note" field) appears incomplete in the context: it ends with "através do Agama Lab." but may need to specify what "through Agama Lab" refers to in the full context. Also verify:

  • Point1 (line 1594): Should include "de amostra" (sample) to match English's "sample project" detail
  • Terminology: Confirm "Padrão" (Default) and "Modo Padrão" usage aligns with the English "Default mode" / "Local mode" terminology established in learnings
  • Structure: Verify note_prefix/note_suffix values at lines 1601-1602 match English equivalents

Based on learnings, the useRemotePolicyStore field should clarify whether "Default" mode means local or remote policy store. Confirm Portuguese line 1597 correctly conveys: when set to Default/Local mode, policies are stored locally on Admin-UI Server and used for Cedarling authorization.

admin-ui/app/locales/en/translation.json (2)

1750-1761: Verify cedarlingConfig note field is complete and clarify "Default mode" terminology per established learnings.

Line 1754 (cedarlingConfig.note) currently reads: "Note: This will help you to create your own cedarling project. You can update roles and permissions using"

This sentence appears incomplete—it ends with "using" without specifying the mechanism (e.g., "using Agama Lab" or "using the refresh button"). The Portuguese translation suggests the intent is "through Agama Lab", so ensure the English version is similarly complete.

Additionally, line 1755 (useRemotePolicyStore) references "Default mode" but per the retrieved learnings, this should explicitly clarify that "Default" now means "Local mode" (policies stored locally on Admin-UI server). The current wording "If set to Default, it will use the Admin-UI storage" implies this, but should be more explicit to prevent user confusion between remote and local storage modes.

Suggested clarification for line 1755:

-      "useRemotePolicyStore": "It is recommended to set it to Default for production. If set to Default, it will use the Admin-UI storage for Cedarling authorization. Enable Default mode and use the refresh button to store or update GitHub policies on the Admin-UI Server.",
+      "useRemotePolicyStore": "It is recommended to enable Local mode (Default) for production. In Local mode, policies are stored on the Admin-UI Server and used for Cedarling authorization. Use the refresh button to sync or update policies from the configured remote URL.",

Confirm with the PR author that the terminology change from "Default mode" to "Local mode" has been finalized, and verify the note field intent per the learnings on sync operations.


660-661: Verify consistency of "mapping" key across localization files.

Line 660 updates menus.securityDropdown.mapping from an implied previous value to "Roles and Permissions". Ensure this key name and value are consistently applied across all localization files (es, fr, pt). The value change from "Mapping" to "Roles and Permissions" should be verified to avoid breaking any hardcoded references in the UI code that depend on the previous English value.

Search the codebase to confirm that no UI code uses the old menu label value for conditional logic or comparisons.

Copy link
Contributor

@duttarnab duttarnab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2025-11-17 at 2 57 31 PM

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-documentation Documentation needs to change as part of issue or PR comp-admin-ui Component affected by issue or PR kind-feature Issue or PR is a new feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(admin-ui): make page UI for cedarling configuration

5 participants